Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Generate dist/dynamic-modules.json for relevant packages #10046

Merged
merged 3 commits into from
Apr 22, 2024
Merged

Generate dist/dynamic-modules.json for relevant packages #10046

merged 3 commits into from
Apr 22, 2024

Conversation

vojtechszocs
Copy link
Contributor

@vojtechszocs vojtechszocs commented Jan 26, 2024 •
edited
Loading

This PR modifies scripts/build-single-packages.js so that a dist/dynamic-modules.json file is generated for each package containing dynamic modules.

# yarn install && yarn build
$ find . -name dynamic-modules.json
./packages/react-core/dist/dynamic-modules.json
./packages/react-icons/dist/dynamic-modules.json
./packages/react-table/dist/dynamic-modules.json
./packages/react-templates/dist/dynamic-modules.json

We generate dynamic-modules.json files to make it easier for consuming projects to share specific PatternFly dynamic modules (instead of sharing the whole package indexes) via webpack module federation.

Each dynamic-modules.json file maps package index exports to their corresponding dynamic modules, for example:

// packages/react-core/dist/dynamic-modules.json
{
  "ASTERISK": "dist/dynamic/helpers/htmlConstants",
  "AboutModal": "dist/dynamic/components/AboutModal",
  "AboutModalProps": "dist/dynamic/components/AboutModal",
  "Accordion": "dist/dynamic/components/Accordion",
  "AccordionContent": "dist/dynamic/components/Accordion",
  "AccordionContentProps": "dist/dynamic/components/Accordion",
  "AccordionExpandableContentBody": "dist/dynamic/components/Accordion",
  "AccordionExpandableContentBodyProps": "dist/dynamic/components/Accordion",
  "AccordionItem": "dist/dynamic/components/Accordion",
  "AccordionItemProps": "dist/dynamic/components/Accordion",
  "AccordionProps": "dist/dynamic/components/Accordion",
  "AccordionToggle": "dist/dynamic/components/Accordion",
  "AccordionToggleProps": "dist/dynamic/components/Accordion",
  // ...
}

The algorithm that parses dynamic module information favors modules with most specific file paths, for example dist/dynamic/components/Wizard/hooks is favored over dist/dynamic/components/Wizard.

Dynamic modules nested under deprecated or next directories are ignored.

Note: this code was ported from openshift/console#13521

@patternfly-build
Copy link
Contributor

patternfly-build commented Jan 26, 2024 •
edited
Loading

Preview: https://patternfly-react-pr-10046.surge.sh

A11y report: https://patternfly-react-pr-10046-a11y.surge.sh

Hyperkid123
Hyperkid123 reviewed Jan 29, 2024
View reviewed changes
scripts/parse-dynamic-modules.js Outdated

/** @type {ts.CompilerOptions} */
const defaultCompilerOptions = {
target: ts.ScriptTarget.ES2020,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if it would be preferred to get some of the compiler options from https://github.com/patternfly/patternfly-react/blob/main/packages/tsconfig.base.json#L9-L12

Copy link
Contributor Author

@vojtechszocs vojtechszocs Apr 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code just parses the relevant info from package-specific build outputs, i.e. there is nothing emitted.

I've hardcoded the above TypeScript compiler option defaults to keep things simple and self-contained.

I see no problem with individual scripts defining their own options assuming they represent a separate build step, i.e. my understanding is that packages/tsconfig.base.json base config is mainly used for building the packages via Rollup.

I can try reusing some options from the base config as suggested, but it shouldn't have any impact on the generated dynamic-modules.json files.

@nicolethoen
Copy link
Contributor

@vojtechszocs @Hyperkid123
I'm noticing here that the generated react-core/dist/dynamic-modules.json file is listing:

...
"Modal": "dist/dynamic/next/components/Modal",
"ModalBox": "dist/dynamic/components/Modal",
...

So i'm not sure this is accounting for next components correctly.

@Hyperkid123
Copy link
Contributor

@vojtechszocs I think the next/deprecated modules should not be listed. There are conflicting names and the API difference will cause unexpected issues.

@vojtechszocs
Copy link
Contributor Author

@Hyperkid123 @nicolethoen

You are right, module paths containing either "next" or "deprecated" should be excluded.

This issue also exists in OpenShift Console dynamic plugin SDK code (dynamic-module-parser.ts).

I'll update this PR as well as relevant Console code.

@vojtechszocs
Copy link
Contributor Author

@nicolethoen PR updated, please have a look.

Dynamic modules nested under deprecated or next directories are ignored.

@dlabaj dlabaj merged commit 9fa6654 into patternfly:main Apr 22, 2024
13 checks passed
@Hyperkid123 Hyperkid123 mentioned this pull request May 3, 2024
Merged
@vojtechszocs vojtechszocs mentioned this pull request May 9, 2024
Merged
@vojtechszocs vojtechszocs mentioned this pull request May 17, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dlabaj dlabaj dlabaj approved these changes

@Hyperkid123 Hyperkid123 Hyperkid123 approved these changes

@nicolethoen nicolethoen nicolethoen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vojtechszocs @patternfly-build @nicolethoen @Hyperkid123 @dlabaj